[lldb][DataFormatter] Format libstdc++ unique_ptr like we do libc++ #146909

Michael137 · 2025-07-03T14:45:02Z

The only difference is that with libc++ the summary string contains the derefernced pointer value. With libstdc++ we currently display the pointer itself, which seems redundant. E.g.,

(std::unique_ptr<int>) iup = 0x55555556d2b0 {
  pointer = 0x000055555556d2b0
}
(std::unique_ptr<std::basic_string<char> >) sup = 0x55555556d2d0 {
  pointer = "foobar"
}

This patch moves the logic into a common helper that's shared between the libc++ and libstdc++ formatters.

After this patch we can combine the libc++ and libstdc++ API tests (see #146740).

Michael137 · 2025-07-03T14:45:39Z

lldb/include/lldb/DataFormatters/FormattersHelpers.h

+/// formatting the rest of the object. Currently this is the case when the
+/// pointer is a nullptr.
+bool DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr,
+                                   const TypeSummaryOptions &options);


The return value is a bit awkward. Happy to consider alternatives.

Is it actually needed though? In my experiments, a normally constructed empty shared pointer will have the __ctrl_ field as null anyway.

I was able to construct a nullptr shared_ptr with a non-empty control field using the subobject constructor (std::shared_ptr<int> si(new int(47)); std::shared_ptr<int> sie(si, nullptr);), but in that case, maybe we actually should show the weak and shared counts?

Good point, don't think the early return is necessary. Let me add that subobject constructor test too

Applied your suggestion (and refactored the weak/strong count logic in the latest commit). Happy to split that up into a separate PR if this is becoming too noisy.

llvmbot · 2025-07-03T14:45:40Z

@llvm/pr-subscribers-lldb

Author: Michael Buch (Michael137)

Changes

The only difference is that with libc++ the summary string contains the derefernced pointer value. With libstdc++ we currently display the pointer itself, which seems redundant. E.g.,

(std::unique_ptr&lt;int&gt;) iup = 0x55555556d2b0 {
  pointer = 0x000055555556d2b0
}
(std::unique_ptr&lt;std::basic_string&lt;char&gt; &gt;) sup = 0x55555556d2d0 {
  pointer = "foobar"
}

This patch moves the logic into a common helper that's shared between the libc++ and libstdc++ formatters.

Full diff: https://github.com/llvm/llvm-project/pull/146909.diff

5 Files Affected:

(modified) lldb/include/lldb/DataFormatters/FormattersHelpers.h (+6)
(modified) lldb/source/DataFormatters/FormattersHelpers.cpp (+22)
(modified) lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp (+2-35)
(modified) lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp (+2-8)
(modified) lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py (+7-7)

diff --git a/lldb/include/lldb/DataFormatters/FormattersHelpers.h b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
index 42699d0a0b1b1..ea77b43e8da39 100644
--- a/lldb/include/lldb/DataFormatters/FormattersHelpers.h
+++ b/lldb/include/lldb/DataFormatters/FormattersHelpers.h
@@ -55,6 +55,12 @@ void AddFilter(TypeCategoryImpl::SharedPointer category_sp,
 
 std::optional<size_t> ExtractIndexFromString(const char *item_name);
 
+/// Returns \c false if the smart pointer formatters shouldn't continue
+/// formatting the rest of the object. Currently this is the case when the
+/// pointer is a nullptr.
+bool DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr,
+                                   const TypeSummaryOptions &options);
+
 Address GetArrayAddressOrPointerValue(ValueObject &valobj);
 
 time_t GetOSXEpoch();
diff --git a/lldb/source/DataFormatters/FormattersHelpers.cpp b/lldb/source/DataFormatters/FormattersHelpers.cpp
index d7b058d91c4a3..022a5d12efae3 100644
--- a/lldb/source/DataFormatters/FormattersHelpers.cpp
+++ b/lldb/source/DataFormatters/FormattersHelpers.cpp
@@ -126,3 +126,25 @@ lldb_private::formatters::GetArrayAddressOrPointerValue(ValueObject &valobj) {
 
   return data_addr.address;
 }
+
+bool lldb_private::formatters::DumpCxxSmartPtrPointerSummary(
+    Stream &stream, ValueObject &ptr, const TypeSummaryOptions &options) {
+  if (ptr.GetValueAsUnsigned(0) == 0) {
+    stream.Printf("nullptr");
+    return true;
+  }
+
+  Status error;
+  ValueObjectSP pointee_sp = ptr.Dereference(error);
+  // FIXME: should we be handling this as an error?
+  if (!pointee_sp || !error.Success())
+    return false;
+
+  if (!pointee_sp->DumpPrintableRepresentation(
+          stream, ValueObject::eValueObjectRepresentationStyleSummary,
+          lldb::eFormatInvalid,
+          ValueObject::PrintableRepresentationSpecialCases::eDisable, false))
+    stream.Printf("ptr = 0x%" PRIx64, ptr.GetValueAsUnsigned(0));
+
+  return false;
+}
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
index 7143089209dd3..995c943ac4da4 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibCxx.cpp
@@ -166,24 +166,8 @@ bool lldb_private::formatters::LibcxxSmartPointerSummaryProvider(
   if (!ptr_sp)
     return false;
 
-  if (ptr_sp->GetValueAsUnsigned(0) == 0) {
-    stream.Printf("nullptr");
+  if (!DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options))
     return true;
-  } else {
-    bool print_pointee = false;
-    Status error;
-    ValueObjectSP pointee_sp = ptr_sp->Dereference(error);
-    if (pointee_sp && error.Success()) {
-      if (pointee_sp->DumpPrintableRepresentation(
-              stream, ValueObject::eValueObjectRepresentationStyleSummary,
-              lldb::eFormatInvalid,
-              ValueObject::PrintableRepresentationSpecialCases::eDisable,
-              false))
-        print_pointee = true;
-    }
-    if (!print_pointee)
-      stream.Printf("ptr = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0));
-  }
 
   if (count_sp)
     stream.Printf(" strong=%" PRIu64, 1 + count_sp->GetValueAsUnsigned(0));
@@ -210,24 +194,7 @@ bool lldb_private::formatters::LibcxxUniquePointerSummaryProvider(
   if (!ptr_sp)
     return false;
 
-  if (ptr_sp->GetValueAsUnsigned(0) == 0) {
-    stream.Printf("nullptr");
-    return true;
-  } else {
-    bool print_pointee = false;
-    Status error;
-    ValueObjectSP pointee_sp = ptr_sp->Dereference(error);
-    if (pointee_sp && error.Success()) {
-      if (pointee_sp->DumpPrintableRepresentation(
-              stream, ValueObject::eValueObjectRepresentationStyleSummary,
-              lldb::eFormatInvalid,
-              ValueObject::PrintableRepresentationSpecialCases::eDisable,
-              false))
-        print_pointee = true;
-    }
-    if (!print_pointee)
-      stream.Printf("ptr = 0x%" PRIx64, ptr_sp->GetValueAsUnsigned(0));
-  }
+  DumpCxxSmartPtrPointerSummary(stream, *ptr_sp, options);
 
   return true;
 }
diff --git a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
index b47ff579b6a5e..e570a4bb1a886 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/LibStdcppUniquePointer.cpp
@@ -156,14 +156,8 @@ bool LibStdcppUniquePtrSyntheticFrontEnd::GetSummary(
   if (!m_ptr_obj)
     return false;
 
-  bool success;
-  uint64_t ptr_value = m_ptr_obj->GetValueAsUnsigned(0, &success);
-  if (!success)
-    return false;
-  if (ptr_value == 0)
-    stream.Printf("nullptr");
-  else
-    stream.Printf("0x%" PRIx64, ptr_value);
+  DumpCxxSmartPtrPointerSummary(stream, *m_ptr_obj, options);
+
   return true;
 }
 
diff --git a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
index 2301e5edfbd90..600a8669c81ba 100644
--- a/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
+++ b/lldb/test/API/functionalities/data-formatter/data-formatter-stl/libstdcpp/unique_ptr/TestDataFormatterStdUniquePtr.py
@@ -31,15 +31,15 @@ def test_with_run_command(self):
         self.assertTrue(frame.IsValid())
 
         self.expect("frame variable nup", substrs=["nup = nullptr"])
-        self.expect("frame variable iup", substrs=["iup = 0x"])
-        self.expect("frame variable sup", substrs=["sup = 0x"])
+        self.expect("frame variable iup", substrs=["iup = 123"])
+        self.expect("frame variable sup", substrs=['sup = "foobar"'])
 
         self.expect("frame variable ndp", substrs=["ndp = nullptr"])
         self.expect(
-            "frame variable idp", substrs=["idp = 0x", "deleter = ", "a = 1", "b = 2"]
+            "frame variable idp", substrs=["idp = 456", "deleter = ", "a = 1", "b = 2"]
         )
         self.expect(
-            "frame variable sdp", substrs=["sdp = 0x", "deleter = ", "a = 3", "b = 4"]
+            "frame variable sdp", substrs=['sdp = "baz"', "deleter = ", "a = 3", "b = 4"]
         )
 
         self.assertEqual(
@@ -106,13 +106,13 @@ def test_recursive_unique_ptr(self):
             substrs=["stopped", "stop reason = breakpoint"],
         )
 
-        self.expect("frame variable f1->fp", substrs=["fp = 0x"])
+        self.expect("frame variable f1->fp", substrs=["fp = Foo @ 0x"])
         self.expect(
-            "frame variable --ptr-depth=1 f1->fp", substrs=["data = 2", "fp = 0x"]
+            "frame variable --ptr-depth=1 f1->fp", substrs=["data = 2", "fp = Foo @ 0x"]
         )
         self.expect(
             "frame variable --ptr-depth=2 f1->fp",
-            substrs=["data = 2", "fp = 0x", "data = 1"],
+            substrs=["data = 2", "fp = Foo @ 0x", "data = 1"],
         )
 
         frame = self.frame()

github-actions · 2025-07-03T14:47:27Z

✅ With the latest revision this PR passed the Python code formatter.

labath · 2025-07-03T15:03:30Z

lldb/source/DataFormatters/FormattersHelpers.cpp

+
+  Status error;
+  ValueObjectSP pointee_sp = ptr.Dereference(error);
+  // FIXME: should we be handling this as an error?


I think it's reasonable to not print anything in this case.

labath · 2025-07-03T15:19:47Z

lldb/include/lldb/DataFormatters/FormattersHelpers.h

+/// formatting the rest of the object. Currently this is the case when the
+/// pointer is a nullptr.
+bool DumpCxxSmartPtrPointerSummary(Stream &stream, ValueObject &ptr,
+                                   const TypeSummaryOptions &options);


Is it actually needed though? In my experiments, a normally constructed empty shared pointer will have the __ctrl_ field as null anyway.

I was able to construct a nullptr shared_ptr with a non-empty control field using the subobject constructor (std::shared_ptr<int> si(new int(47)); std::shared_ptr<int> sie(si, nullptr);), but in that case, maybe we actually should show the weak and shared counts?

The only difference is that with libc++ the summary string contains the derefernced pointer value. With libstdc++ we currently display the pointer itself, which seems redundant. E.g., ``` (std::unique_ptr<int>) iup = 0x55555556d2b0 { pointer = 0x000055555556d2b0 } (std::unique_ptr<std::basic_string<char> >) sup = 0x55555556d2d0 { pointer = "foobar" } ``` This patch moves the logic into a common helper that's shared between the libc++ and libstdc++ formatters.

…g count printing

labath

Looks good. Just noting I'm kind of surprised that these pointers have a weak_count of 1. I see it's a preexisting issue, and it may make sense if you know the implementation, but as a user, I'm surprised.

Michael137 · 2025-07-04T08:18:19Z

Looks good. Just noting I'm kind of surprised that these pointers have a weak_count of 1. I see it's a preexisting issue, and it may make sense if you know the implementation, but as a user, I'm surprised.

Yea the counts confused me a bit. I'll have a look at that separately.

…lvm#146909) The only difference is that with libc++ the summary string contains the derefernced pointer value. With libstdc++ we currently display the pointer itself, which seems redundant. E.g., ``` (std::unique_ptr<int>) iup = 0x55555556d2b0 { pointer = 0x000055555556d2b0 } (std::unique_ptr<std::basic_string<char> >) sup = 0x55555556d2d0 { pointer = "foobar" } ``` This patch moves the logic into a common helper that's shared between the libc++ and libstdc++ formatters. After this patch we can combine the libc++ and libstdc++ API tests (see llvm#146740). (cherry picked from commit a89e232)

Michael137 requested a review from labath July 3, 2025 14:45

Michael137 requested a review from JDevlieghere as a code owner July 3, 2025 14:45

llvmbot added the lldb label Jul 3, 2025

Michael137 commented Jul 3, 2025

View reviewed changes

labath reviewed Jul 3, 2025

View reviewed changes

Michael137 added 5 commits July 3, 2025 16:32

fixup! python format

63066cc

fixup! remove early-return for empty shared_ptrs; refactor weak/stron…

84de12d

…g count printing

fixup! uncomment subobject constructor test

bd20056

fixup! python format

3117231

labath approved these changes Jul 4, 2025

View reviewed changes

Michael137 merged commit a89e232 into llvm:main Jul 4, 2025
9 checks passed

Michael137 deleted the lldb/libstdcpp-unique_ptr-summary branch July 4, 2025 08:18

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lldb][DataFormatter] Format libstdc++ unique_ptr like we do libc++ #146909

[lldb][DataFormatter] Format libstdc++ unique_ptr like we do libc++ #146909

Uh oh!

Michael137 commented Jul 3, 2025 •

edited

Loading

Uh oh!

Michael137 Jul 3, 2025

Uh oh!

labath Jul 3, 2025 •

edited

Loading

Uh oh!

Michael137 Jul 3, 2025

Uh oh!

Michael137 Jul 3, 2025

Uh oh!

llvmbot commented Jul 3, 2025

Uh oh!

github-actions bot commented Jul 3, 2025 •

edited

Loading

Uh oh!

labath Jul 3, 2025

Uh oh!

labath Jul 3, 2025 •

edited

Loading

Uh oh!

labath left a comment

Uh oh!

Michael137 commented Jul 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

[lldb][DataFormatter] Format libstdc++ unique_ptr like we do libc++ #146909

[lldb][DataFormatter] Format libstdc++ unique_ptr like we do libc++ #146909

Uh oh!

Conversation

Michael137 commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Michael137 Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

labath Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Michael137 Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Michael137 Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

llvmbot commented Jul 3, 2025

Uh oh!

github-actions bot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

labath Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

labath Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

labath left a comment

Choose a reason for hiding this comment

Uh oh!

Michael137 commented Jul 4, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Michael137 commented Jul 3, 2025 •

edited

Loading

labath Jul 3, 2025 •

edited

Loading

github-actions bot commented Jul 3, 2025 •

edited

Loading

labath Jul 3, 2025 •

edited

Loading